[cDAC] Restore Frame-walk fallback in Thread.GetContext#128518
Open
Copilot wants to merge 3 commits into
Open
Conversation
…dContext is unavailable Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes the cDAC IThread.GetContext implementation to fall back to deriving a thread context from the thread’s explicit Frame chain when the data target can’t provide an OS thread context, and updates the Thread data contract design doc accordingly.
Changes:
- Update
Thread_1.GetContextto return the OS context whenTryGetThreadContextsucceeds, otherwise derive a context by walking the thread’sFramechain. - Add a helper (
GetContextFromFrames) that selects anInterpreterFramecontext (viaInterpMethodContextFrame) or the first frame producing non-zero SP/IP. - Update
docs/design/datacontracts/Thread.mdpseudocode to document the fallback behavior.
Note: I did not run build/tests as part of this review.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs | Adds frame-walk fallback path for GetContext when TryGetThreadContext doesn’t provide a context. |
| docs/design/datacontracts/Thread.md | Documents the updated GetContext behavior and fallback selection rules. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 3
tommcdon
approved these changes
May 24, 2026
Member
tommcdon
left a comment
tommcdon
left a comment
There was a problem hiding this comment.
LGTM! Both native and managed implementations look functionally equivalent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
E_NOTIMPLFrame-walk fallback removed in 5391e9e is still needed for data targets that don't implementGetThreadContext. The native side is being restored in a separate PR; this change brings the equivalent path to cDAC.Changes
Thread_1.GetContext: WhenTryGetThreadContextreturns false, walk the thread's Frame chain (modeled afterIStackWalk.GetFrames) instead of throwingInvalidOperationException.InterpreterFrame, fill the context from the topInterpMethodContextFramevia the existingUpdateContextFromCurrentFrame(which callsSetContextToInterpMethodContextFrame).RedirectedThreadFrame,InlinedCallFrame,DynamicHelperFrame), settingRawContextFlags = FullContextFlagsto mark SP/PC/FP as valid. This mirrors the nativeDT_CONTEXT_CONTROL [| DT_CONTEXT_INTEGER]intent without per-architecture branching.docs/design/datacontracts/Thread.md: Updated theGetContextpseudocode to document the new fallback.